Skip to content

fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration#1329

Merged
johntmyers merged 2 commits into
NVIDIA:mainfrom
russellb:fix/1169-dns-exfiltration-mechanistic-mapper
May 15, 2026
Merged

fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration#1329
johntmyers merged 2 commits into
NVIDIA:mainfrom
russellb:fix/1169-dns-exfiltration-mechanistic-mapper

Conversation

@russellb
Copy link
Copy Markdown
Contributor

Summary

The mechanistic mapper resolved denied hostnames via DNS to check whether they mapped to private IPs (for populating allowed_ips in proposals). This leaked the denied hostname through DNS queries even though the HTTP connection was blocked by policy — a data exfiltration vector.

Related Issue

Closes #1169

Changes

  • Remove resolve_allowed_ips_if_private() and all DNS resolution from the mechanistic mapper
  • Proposals no longer include allowed_ips; private IP access requires explicit user configuration
  • Convert generate_proposals() from async to sync (no longer needs tokio for DNS)
  • Update tests from #[tokio::test] async to #[test] sync
  • Remove is_internal_ip tests that belonged in openshell-core, not the mapper

If a user applies a proposed rule and the host resolves to a private IP, the proxy's SSRF defense denies the connection. That denial flows back through the aggregator, and the user can then explicitly configure allowed_ips in their policy. This two-step flow is also more explicit about private IP access, which is desirable given #1245.

Testing

  • mise run pre-commit passes
  • Unit tests updated (async → sync, removed is_internal_ip tests)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 12, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

…t data exfiltration

The mechanistic mapper resolved denied hostnames via DNS to check whether
they mapped to private IPs (for populating allowed_ips in proposals).
This leaked the denied hostname through DNS queries even though the HTTP
connection was blocked by policy — a data exfiltration vector.

Remove resolve_allowed_ips_if_private() entirely. Proposals no longer
include allowed_ips. If a user applies a proposed rule and the host
resolves to a private IP, the proxy's SSRF defense denies the connection.
That denial flows back through the aggregator, and the user can then
explicitly configure allowed_ips in their policy. This two-step flow is
also more explicit about private IP access, which is desirable given NVIDIA#1245.

Closes NVIDIA#1169

Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
@russellb russellb force-pushed the fix/1169-dns-exfiltration-mechanistic-mapper branch from e7acd65 to 1de8e7a Compare May 12, 2026 15:40
Copy link
Copy Markdown
Collaborator

@johntmyers johntmyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small follow-ups from review:

  • In crates/openshell-sandbox/src/mechanistic_mapper.rs, the SSRF security note still says approving the proposal “bypasses internal-IP safety checks.” Since proposals no longer include allowed_ips, approving one does not bypass SSRF anymore. Consider changing this to something like: “This connection was blocked by SSRF protection. Private IP access requires an explicit allowed_ips policy entry.”

  • architecture/security-policy.md still says is_internal_ip is used by the mechanistic mapper to detect when allowed_ips should be populated in proposals. That looks stale after this change. It should describe is_internal_ip as runtime proxy SSRF classification only, or otherwise note that mapper proposals intentionally do not populate allowed_ips.

Update the mechanistic mapper SSRF security note to reflect that
proposals no longer populate allowed_ips — approving a proposal does
not bypass SSRF protection. Clarify the two-step approval flow in
architecture/security-policy.md.

Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
@russellb
Copy link
Copy Markdown
Contributor Author

Addressed both items from review:

Item 1 — SSRF security note (mechanistic_mapper.rs:279): Updated the message from "Allowing it bypasses internal-IP safety checks" to "Private IP access requires an explicit allowed_ips policy entry." This matches the two-step approval flow: proposals never include allowed_ips, so approving one does not bypass SSRF.

Item 2 — Architecture doc (security-policy.md): I couldn't find an existing is_internal_ip reference in this file or any other architecture doc, but the Policy Advisor section was missing context about proposals intentionally omitting allowed_ips. Added a paragraph documenting the two-step flow: proposals omit allowed_ips, the proxy's SSRF classification blocks private-IP connections at runtime, and the operator must add an explicit allowed_ips entry to permit them.

@russellb russellb requested a review from johntmyers May 15, 2026 12:06
@johntmyers
Copy link
Copy Markdown
Collaborator

/ok to test 83eab3a

@johntmyers johntmyers merged commit 9a7c0df into NVIDIA:main May 15, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DNS-based data exfiltration bypasses sandbox network policy

2 participants